Add ms suffix support to DurationTag - #125
Conversation
| return new DurationTag(numVal * 0.05); | ||
| } | ||
| else if (string.endsWith("ms")) { | ||
| return new DurationTag(numVal / 1000.0); |
There was a problem hiding this comment.
Should stick with the conversion pattern that already exists ("* 0.001" rather than "/ 1000.0").
| else if (string.endsWith("t")) { | ||
| return new DurationTag(numVal * 0.05); | ||
| } | ||
| else if (string.endsWith("ms")) { |
There was a problem hiding this comment.
This code is wrong; this branch cannot ever be hit
| if (quickMap == null) { | ||
| val.getMap(); | ||
| val.string = null; | ||
| modified = true; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Since this isn't related to the same issue, it should be in a separate PR.
b24c0a6 to
664acd2
Compare
| } | ||
| } | ||
| String numericString = Character.isDigit(string.charAt(string.length() - 1)) ? string : string.substring(0, string.length() - 1); | ||
| String numericString = Character.isDigit(string.charAt(string.length() - 1)) ? string : string.substring(0, string.endsWith("ms") ? string.length() - 2 : string.length() - 1); |
There was a problem hiding this comment.
(minor) tern contains the same string.length() factor on both sides, can be simplified
|
There's a lot of secondary ramifications. For example, |
|
I made sure to as thoroughly test these as I could; |
| // --> | ||
| tagProcessor.registerStaticTag(DurationTag.class, DurationTag.class, "sub", (attribute, object, secondVal) -> { | ||
| return new DurationTag(object.getTicks() - secondVal.getTicks()); | ||
| return new DurationTag(java.math.BigDecimal.valueOf(object.seconds).subtract(java.math.BigDecimal.valueOf(secondVal.seconds)).doubleValue()); |
There was a problem hiding this comment.
I don't think this... does anything of value with BigDecimal?
There was a problem hiding this comment.
before this commit, two of my close-edge tags parsed as 1.000000000001ms or something crazy; I've lost the log since
|
|
||
| public RepeatingSchedulable(Runnable runnable, float fireRate) { | ||
| run = runnable; | ||
| if (fireRate > 0 && fireRate < 0.05f) { |
There was a problem hiding this comment.
this is very questionable and probably wrong
There was a problem hiding this comment.
i'll work on this; i expected if somebody asks RepeatingSchedulable to repeat faster than one tick that it should by default change their requested interval to one tick
3 participants
Summary
this pull request adds millisecond suffix support to the
DurationTag, allowing scripters to define rapid durations nicely (eg:<duration[3ms]>). this removes the need to divide define millisecondsProblem
when defining durations, we previously lacked a native millisecond suffix; and when checking for suffixes, there was a structural issue where
msstrings were being trimmed by only one character which would fail to parse right. if placed wrongly,"3ms".endsWith("s")would trigger as seconds instead. this fixes the length-check to strip two characters forms, scales the numeric value by0.001to maintain consistency with our existing multiplier standard, and uses themsbranch before seconds to avoid problemsTesting
500ms,250.5ms)DurationTag.in_secondsused various examples:
/ex -q narrate "1ms = <duration[1ms].in_milliseconds>ms"/ex -q narrate "500ms = <duration[500ms].in_milliseconds>ms"/ex -q narrate "1000ms = <duration[1000ms].in_milliseconds>ms"/ex -q narrate "500ms = <duration[500ms].in_seconds>s"/ex -q narrate "250.5ms = <duration[250.5ms].in_milliseconds>ms"here's a startup log referencing this test with tags parsed for verification:
https://paste.denizenscript.com/View/141404
a working build of my latest successful build which has no errors can be built with this branch or found at:
https://files.behr.dev/file-share/Denizen-1.3.3-bUnknown-CUSTOM-ms-durationtag.jar